Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

robot-simulator: apply "input" policy #1163

Merged
merged 2 commits into from
Jan 31, 2018
Merged

Conversation

rpottsoh
Copy link
Member

per #996

@rpottsoh rpottsoh self-assigned this Jan 30, 2018
Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider it acceptable to have position and direction be properties of input, and remove the robot altogether. I'll leave it up to y'all.

@ErikSchierboom
Copy link
Member

I would consider it acceptable to have position and direction be properties of input, and remove the robot altogether.

I think I agree. This is also already the case for some of the expected values.

"robot" deemed unnecessary, replaced by "input".
@rpottsoh
Copy link
Member Author

@petertseng lately as I have been applying #996 to exercise testdata I have been receiving an email from Travis CI letting me know the status of the checks it has made. This is a recent behavior that I have noticed. I am not sure if I like it or not. I wanted to point out that for the second commit of this PR I did not receive an email from Travis CI. Should I be expecting an email for every commit or just this first? Should I even be receiving email from Travis CI? I apologize in advance if you are not the person to whose attention I should bring this to.

@petertseng
Copy link
Member

I have no idea about Travis. Apology accepted.

@petertseng
Copy link
Member

petertseng commented Jan 30, 2018

I'm going to guess you're getting emails because you are making your branches on Exercism's copy of the repo, instead of on your fork. In that case, I have observed getting an email on:

  • First time the branch is created
  • State transition from pass to fail
  • State transition from fail to fail
  • State transition from fail to pass

I have observed not getting an email on:

  • State transition from pass to pass

I have not yet discovered a way to change Travis's behaviour in this regard. Either way, since making branch on Exercism's copy of the repo is behaviour that I do not condone (exercism/discussions#174 (comment)), I can be of no further help in this matter. If you wish not to receive emails, what I suggest to try to do is to disable Travis CI for your fork and then make branches on your fork. I'm not sure that will make you not receive emails. If you wanted to be sure, you would have to consult the Travis documentation or ask those knowledgeable about Travis.

@rpottsoh
Copy link
Member Author

@petertseng your guess is correct. I was preparing a different comment, but had to break for lunch, since then your comment came in that sums up my observation exactly.

When I create a branch off of exercism/master from which I submit a PR, Travis performs 2 checks. If I submit the PR from a branch off of my fork Travis only performs one check. I do not receive an email from Travis for the PR that originates from my fork so I have determined that the continuous-integration/travis-ci/push check is triggering the email because this check is not performed for PRs that do not originate from a branch off of exercism/master.

regarding receiving email, if I simply go back to submitting PRs from my fork the emails will cease. I am left wondering why sometimes 2 checks are made vs. 1.

@petertseng
Copy link
Member

petertseng commented Jan 30, 2018

2 checks are made vs. 1.

One build for every PR made to exercism, regardless of whether the branch backing the PR is on a fork: https://travis-ci.org/exercism/problem-specifications/pull_requests, because "Build pushed pull branches" is enabled in https://travis-ci.org/exercism/problem-specifications/settings

One build for every branch on exercism's copy of the repo (and therefore by definition not on any forks): https://travis-ci.org/exercism/problem-specifications/branches, because "Build pushed branches" is enabled in https://travis-ci.org/exercism/problem-specifications/settings

Any PR backed by a branch on exercism's copy of the repo thus gets two builds, as a logical consequence.

@rpottsoh
Copy link
Member Author

rpottsoh commented Jan 30, 2018 via email

@rpottsoh rpottsoh merged commit 3763965 into master Jan 31, 2018
@rpottsoh rpottsoh deleted the updateRobot-Simulator branch January 31, 2018 14:36
@petertseng
Copy link
Member

petertseng commented Jan 31, 2018

instructions is an input, but it was not moved to the input object.

@rpottsoh
Copy link
Member Author

instructions is an input, but it was not moved to the input object.

Oops. Thanks for catching that @petertseng. I'll take care of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants